Skip to content

Conversation

@heroku-johnny
Copy link
Contributor

Description

Work Item: W-19048403 [terraform-provider-heroku] Fir Compatibility: Builds

This PR implements Fir generation validation for the heroku_build resource, preventing incompatible buildpack configurations for Fir apps while maintaining full backward compatibility with traditional Cedar buildpack workflows.

Key Features

  • Build validation: Automatic blocking of buildpacks for Fir generation apps
  • CNB compatibility: Clear guidance directing users to Cloud Native Buildpacks via project.toml
  • Plan & apply validation: Two-layer validation for comprehensive error handling
  • Consistent UX: Aligns with Heroku CLI behavior preventing silent API failures

Changes

Feature Matrix Enhancement

  • Add build-specific features to generation matrix (buildpacks vs cloud_native_buildpacks)
  • Cedar generation: Supports traditional buildpack URLs and configurations
  • Fir generation: Blocks traditional buildpacks, supports Cloud Native Buildpacks via project.toml

Build Resource Validation

  • Add plan-time validation with dependency-aware logic in CustomizeDiff
  • Add apply-time validation preventing misleading API "success"
  • Implement comprehensive error handling with actionable user guidance
  • Add acceptance tests with real API validation (2 test scenarios)
  • Update documentation with generation examples and CNB migration guidance

Usage Examples

# Cedar generation - traditional buildpacks (works as before)
resource "heroku_build" "cedar_build" {
  app_id     = heroku_app.cedar_app.id
  buildpacks = ["heroku/nodejs", "heroku/procfile"]
  
  source {
    path = "path/to/source"
  }
}

# Fir generation - Cloud Native Buildpacks only
resource "heroku_build" "fir_build" {
  app_id = heroku_app.fir_app.id  # App in Fir space
  # No buildpacks - configured via project.toml in source
  
  source {
    path = "path/to/cnb/source"  # Contains project.toml
  }
}

# This will FAIL with clear error:
resource "heroku_build" "invalid" {
  app_id     = heroku_app.fir_app.id
  buildpacks = ["heroku/nodejs"]  # ❌ Error at apply time
  
  source {
    path = "path/to/source"
  }
}

Error Handling

Apply-time validation prevents invalid configurations:
"Error: buildpacks cannot be specified for fir generation apps. Use project.toml to configure Cloud Native Buildpacks instead. See: https://devcenter.heroku.com/articles/using-multiple-buildpacks-for-an-app"

Test Coverage

  • Comprehensive unit tests for feature matrix validation (4 test cases)
  • Acceptance test scenarios with real API validation:
    • Cedar build with buildpacks (success)
    • Fir build without buildpacks (success)
    • Fir build with buildpacks (apply-time error)
  • Manual validation confirmed end-to-end error handling

Documentation Updates

  • Generation compatibility section with Cedar vs Fir comparison
  • Cloud Native Buildpacks guidance with project.toml examples
  • Updated examples showing both Cedar and Fir generation usage
  • Clear migration path from traditional buildpacks to CNB

Technical Implementation

  • DRY validation logic: Shared helper functions for consistent behavior
  • Two-layer validation: Plan-time (when possible) + apply-time (guaranteed)
  • Dependency handling: Graceful plan-time behavior when app doesn't exist yet
  • API behavior fix: Prevents misleading "success" when buildpacks are silently ignored

Breaking Changes: None
Backward Compatible: Yes (no generation field added to build resource)
Foundation: Extends existing feature matrix framework from previous fir compatibility PRs
Dependencies: Requires feature matrix foundation and app generation detection from previous PRs

Validates that buildpacks cannot be specified for Fir generation apps, which use Cloud Native Buildpacks configured via project.toml instead.

- Add plan-time and apply-time validation for buildpack compatibility
- Add comprehensive unit and acceptance tests
- Update documentation with generation-specific examples
- Add build features to generation feature matrix
- Remove standalone TestAccHerokuBuild_Generation test
- Add testStep_AccHerokuBuild_Generation_FirValid and FirInvalid helpers
- Integrate build tests into existing TestAccHerokuSpace_Fir
- Follow existing test pattern for efficiency and consistency
- Use unique resource names between test steps to prevent conflicts
- fir_build_app_valid vs fir_build_app_invalid
- Different app name patterns for each test step
- Fixes 'plan was not empty' acceptance test failure
@heroku-johnny heroku-johnny merged commit 867294d into fir-compatibility Sep 18, 2025
1 check passed
mars added a commit that referenced this pull request Sep 29, 2025
* Add Fir generation support for Heroku Private Spaces  (#401)

* Include .cursor in gitignore

* Add foundational feature matrix system for generation support

- Implement IsFeatureSupported() helper function for Cedar/Fir generation differences
- Add feature matrix tracking space capabilities across generations
- Include comprehensive test coverage with 14 test cases
- Cedar generation: supports all space features including shield spaces
- Fir generation: supports private spaces only, shield spaces unsupported
- Foundation for graceful handling of generation-specific feature differences

* Add Fir generation support for Private Spaces

- Add generation field to heroku_space resource with cedar/fir validation
- Implement shield feature validation blocking Fir+Shield combinations
- Add comprehensive CRUD validation (Create errors, Read warnings)
- Enhance feature matrix with complete space feature coverage
- Add extensive test suite: 24 tests including acceptance tests
- Update documentation with generation examples and guidance
- Maintain backward compatibility with cedar default
- Foundation for incremental space feature validation

Resolves core deliverables for Fir Private Space compatibility.
Generated shield spaces require cedar generation.
ForceNew ensures generation cannot be changed after creation.

* Improve generation validation with plan-time error checking

- Add CustomizeDiff function for shield+generation validation during plan phase
- Users now get immediate feedback during 'terraform plan' instead of waiting for apply
- Remove redundant validation from Create function since CustomizeDiff handles it
- Better UX: clear error messages during planning prevent surprises during apply

* Document outbound IPs limitation for Fir generation

- Add note to space documentation that outbound IP management
  is not supported for fir generation spaces
- Provides clear guidance to users about generation differences

* Optimize Fir space acceptance tests to use single space pattern

- Consolidate TestAccHerokuSpace_Generation from 3 spaces to 1 space
- Create new TestAccHerokuSpace_Fir following efficient single-space pattern
- Remove TestAccHerokuSpace_GenerationForceNew (redundant with generation change test)
- Add Fir-specific validation test steps for VPN/inbound/peering failures
- Reduces space creations from 6 to 2 (~70% faster test execution)
- Follows established pattern from main TestAccHerokuSpace function

* Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update heroku/resource_heroku_space.go

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

Update docs/resources/space.md

Co-authored-by: Sandy Lai <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

* Pass generation field to Heroku API for space creation

- Add generation field to SpaceCreateOpts when creating spaces
- Fix resourceHerokuSpaceRead to properly read generation from API response
- Ensure users get the generation they specify instead of defaulting to Cedar
- Add debug logging for space creation with generation
- Tested: Cedar space creation confirmed working, Fir space API correctly validates generation

* Remove default value for CIDR from spaces

* Include the fir feature branch for acceptance test runs

* Remove generation name check

* Remove unused test steps

---------

Signed-off-by: Johnny Winn <[email protected]>
Co-authored-by: Sandy Lai <[email protected]>

* Add Fir generation support for Heroku Apps (#402)

* Add foundational feature matrix system for generation support

- Implement IsFeatureSupported() helper function for Cedar/Fir generation differences
- Add feature matrix tracking space capabilities across generations
- Include comprehensive test coverage with 14 test cases
- Cedar generation: supports all space features including shield spaces
- Fir generation: supports private spaces only, shield spaces unsupported
- Foundation for graceful handling of generation-specific feature differences

* Add generation support to apps with CNB validation and acceptance tests

* Refactor app generation to be computed from space

Major UX improvement: generation is now automatically determined
from the space the app is deployed to, rather than user-configured.

Changes:
- App 'generation' field is now computed-only (was optional)
- Apps in Fir spaces automatically get generation = 'fir'
- Apps in Cedar spaces (or no space) get generation = 'cedar'

Core Improvements:
- Fix CNB buildpack errors by conditionally querying based on generation
- Smart buildpack handling: skip traditional queries for Fir apps
- Automatic space generation detection via SpaceInfo API
- Better error prevention and more intuitive configuration

Test Improvements:
- Consolidate acceptance tests with single space approach
- Remove invalid validation tests (no longer user-configurable)
- Update test expectations for computed generation behavior
- Maintain comprehensive coverage for both generations

Documentation Updates:
- Update examples to show space-based generation approach
- Clarify that generation is computed from space deployment
- Improve migration guide with proper space-first workflow
- Update argument/attribute references for computed field

* Update docs/resources/app.md

Co-authored-by: Mars Hall <[email protected]>
Signed-off-by: Johnny Winn <[email protected]>

* Optimize app generation detection by using AppInfo response

- Use app.Generation.Name directly from AppInfo response instead of making separate SpaceInfo API call
- Remove unnecessary getSpaceGeneration() function and SpaceInfo API request
- Maintains same functionality with better performance and fewer API calls
- Addresses PR feedback about unnecessary API requests

---------

Signed-off-by: Johnny Winn <[email protected]>
Co-authored-by: Mars Hall <[email protected]>

* Add Fir generation validation for `heroku_build` resource (#406)

* Add Fir generation validation for heroku_build resource

Validates that buildpacks cannot be specified for Fir generation apps, which use Cloud Native Buildpacks configured via project.toml instead.

- Add plan-time and apply-time validation for buildpack compatibility
- Add comprehensive unit and acceptance tests
- Update documentation with generation-specific examples
- Add build features to generation feature matrix

* refactor: Use shared Fir space for build generation tests

- Remove standalone TestAccHerokuBuild_Generation test
- Add testStep_AccHerokuBuild_Generation_FirValid and FirInvalid helpers
- Integrate build tests into existing TestAccHerokuSpace_Fir
- Follow existing test pattern for efficiency and consistency

* Resolve resource name conflicts in build generation tests

- Use unique resource names between test steps to prevent conflicts
- fir_build_app_valid vs fir_build_app_invalid
- Different app name patterns for each test step
- Fixes 'plan was not empty' acceptance test failure

* Add Fir generation support to heroku_app_release (#407)

* feat: enhance pipeline coupling errors for mixed generations (#409)

- Add contextual error messages when coupling apps from different generations
- Include app name and generation in error output for better UX
- Update pipeline documentation with generation compatibility requirements

Addresses mixed-generation pipeline error handling work item.

* Add new Pipeline Promotion resource (#410)

* Add heroku_pipeline_promotion resource

* Update heroku-go API client to add support for Promoting by Release ID

* Enable release_id support for pipeline promotions

- Flow team has added Promotion#release_id API support
- Remove error check that blocked release_id usage
- Add release_id to API call when provided
- Update documentation with release_id examples and remove outdated notes
- Remove obsolete unit test for release_id validation
- Pipeline promotions now support both latest and specific release promotion
- Validated with local terraform config - both promotion types work perfectly

* Make release_id required for pipeline promotions

- Change release_id from Optional to Required in schema
- Remove conditional logic in Create function since release_id is always present
- Update documentation to reflect required field
- Update unit test to validate release_id as required field
- Simplify examples and descriptions to focus on specific release promotion
- This provides clearer, more predictable behavior for users

* Fix flaky VPN connection test - make tunnel count flexible

- Change from exact tunnel count check (tunnels.# = 2) to attribute existence check
- VPN tunnel provisioning can be delayed in test environments
- Still validates VPN connection creation and tunnels field presence
- Resolves consistent test failures across multiple PRs where tunnels.# expected 2 got 0
- This test failure was blocking unrelated PRs (telemetry drains, pipeline promotions)

---------

Co-authored-by: Mars Hall <[email protected]>

* Add new Telemetry Drain resource (#411)

* Add new heroku_telemetry_drain resource for fir generation

* Fix telemetry drain acceptance test configuration

- Changed space-scoped drain from logs-only to traces+metrics
- Updated endpoint from otlp to otlphttp format
- Resolves API parameter validation error in acceptance tests

* Remove space-scoped drain from acceptance test

- Space-scoped telemetry drains encounter API validation issues
- Simplified to test only app-scoped drains which work reliably
- Acceptance test now passes both locally and on CI

* Make headers required for telemetry drains and add space-scoped drain test

- API testing revealed headers are required for all telemetry drains
- Updated schema to make headers required instead of optional
- Added space-scoped telemetry drain back to acceptance test with headers
- Updated documentation to reflect headers requirement
- Both app-scoped and space-scoped drains now work correctly

* Fix acceptance test issues for telemetry drains

- Remove problematic Fir build invalid test from space test to avoid app creation limits
- Fix unit test to expect headers field as required (matching schema)
- Update build test error pattern to match full validation message
- Telemetry drain tests are the focus of this branch, build tests are covered elsewhere

* Fix flaky VPN connection test - make tunnel count flexible

- Change from exact tunnel count check (tunnels.# = 2) to attribute existence check
- VPN tunnel provisioning can be delayed in test environments
- Still validates VPN connection creation and tunnels field presence
- Resolves consistent test failures across multiple PRs where tunnels.# expected 2 got 0
- This test failure was blocking unrelated PRs (telemetry drains, pipeline promotions)

* Doc updates for Fir/CNBs (#413)

* Update app.md

Signed-off-by: Sandy Lai <[email protected]>

* Update app.md

Signed-off-by: Sandy Lai <[email protected]>

* Update app.md

Signed-off-by: Sandy Lai <[email protected]>

* Update build.md

Update for style, de-emphasize CNBs=Fir, add Optional to buildpacks text definition for CNBs, change mentions of slug where appropriate to artifact, add note that slug_id is only for apps with classic buildpacks. 

Signed-off-by: Sandy Lai <[email protected]>

* Update build.md

fix typo

Signed-off-by: Sandy Lai <[email protected]>

* Plain english guidelines updates app.md

Signed-off-by: Sandy Lai <[email protected]>

* rephrase note as positive app.md

Signed-off-by: Sandy Lai <[email protected]>

* Update space.md to use plain technical English

Signed-off-by: Sandy Lai <[email protected]>

* Update space.md

Signed-off-by: Sandy Lai <[email protected]>

* Update pipeline.md to plain technical English, change slug to artifacts

Signed-off-by: Sandy Lai <[email protected]>

* Peer review terraform docs (#412)

* Peer review on app.md

Signed-off-by: Helen Cheng <[email protected]>

* Peer review for build.md

Signed-off-by: Helen Cheng <[email protected]>

* Peer review for pipeline.md

Signed-off-by: Helen Cheng <[email protected]>

* Peer review for space.md

Signed-off-by: Helen Cheng <[email protected]>

---------

Signed-off-by: Helen Cheng <[email protected]>

* Update app.md

Signed-off-by: Sandy Lai <[email protected]>

* Add revised telemetry_drain.md

Signed-off-by: Sandy Lai <[email protected]>

* Add reviewed pipeline_promotion.md

Signed-off-by: Sandy Lai <[email protected]>

* Delete docs/pipeline_promotion.md

Signed-off-by: Sandy Lai <[email protected]>

* Create pipeline_promotion.md

Signed-off-by: Sandy Lai <[email protected]>

* Add Cedar-only note to app_release.md

Signed-off-by: Sandy Lai <[email protected]>

* Update note about classic buildpacks app_release.md

Signed-off-by: Sandy Lai <[email protected]>

* Add classic buildpacks-only note to slug.md

Also conform to style guidelines

Signed-off-by: Sandy Lai <[email protected]>

* Add Cedar-only note to space_inbound_ruleset.md

Signed-off-by: Sandy Lai <[email protected]>

* Add Cedar-only note space_peering_connection_accepter.md

And changes to conform to style guidelines

Signed-off-by: Sandy Lai <[email protected]>

* Update pipeline.md

Signed-off-by: Sandy Lai <[email protected]>

* Add classic buildpack note to app.md

and changes to conform to style guidelines

Signed-off-by: Sandy Lai <[email protected]>

* Add Cedar-only note review_app_config.md

also changes to conform to style guidelines

Signed-off-by: Sandy Lai <[email protected]>

* Change slug to build artifacts in pipeline_coupling.md

Signed-off-by: Sandy Lai <[email protected]>

* Update CIDR attributes to say only for Cedar in space.md

Signed-off-by: Sandy Lai <[email protected]>

* Fix formatting docs/data-sources/app.md

Co-authored-by: Mars Hall <[email protected]>
Signed-off-by: Sandy Lai <[email protected]>

* Peer review for terraform docs (#414)

* Peer review pipeline_promotion.md

Signed-off-by: Helen Cheng <[email protected]>

* Update telemetry_drain.md

Signed-off-by: Helen Cheng <[email protected]>

* Correct note format pipeline_promotion.md

Signed-off-by: Sandy Lai <[email protected]>

---------

Signed-off-by: Helen Cheng <[email protected]>
Signed-off-by: Sandy Lai <[email protected]>
Co-authored-by: Sandy Lai <[email protected]>

* Fix note formatting app.md

Signed-off-by: Sandy Lai <[email protected]>

* Fix note formatting in build.md

Signed-off-by: Sandy Lai <[email protected]>

* Update space.md

Signed-off-by: Sandy Lai <[email protected]>

---------

Signed-off-by: Sandy Lai <[email protected]>
Signed-off-by: Helen Cheng <[email protected]>
Co-authored-by: Helen Cheng <[email protected]>
Co-authored-by: Mars Hall <[email protected]>

---------

Signed-off-by: Johnny Winn <[email protected]>
Signed-off-by: Sandy Lai <[email protected]>
Signed-off-by: Helen Cheng <[email protected]>
Signed-off-by: Mars Hall <[email protected]>
Co-authored-by: Sandy Lai <[email protected]>
Co-authored-by: Mars Hall <[email protected]>
Co-authored-by: Mars Hall <[email protected]>
Co-authored-by: Helen Cheng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants